-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KDT5 10조 과제 제출 #11
base: main
Are you sure you want to change the base?
KDT5 10조 과제 제출 #11
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
총평
어려운 상황에서 적은 인원으로 프로젝트를 착실히 수행하시느라 수고하셨습니다.
아쉬운 점
배포된 프로덕트의 코드에 console.log가 너무 많이 남아있습니다.
코드 상에서는 console.log가 없도록 하는 것이 좋습니다.
또한, 어떤 컴포넌트에서는 css를 사용하고 어떤 컴포넌트에서는 css in js를 사용하는 것이 아쉽습니다.
프로젝트에서는 컨벤션을 맞추는 것이 아주 중요합니다.
state를 효율적으로 관리하지 못하는 부분이 조금 있습니다.
관리하는 state의 수를 최소화 하는 것이 유지보수 측면에서 유리합니다.
모두 고생하셨습니다.
onClick={e => { | ||
e.preventDefault() | ||
}}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
특별한 경우가 아니라면 a 태그에 onClick은 달지 않는 것이 좋습니다.
접근성 측면에서 좋지 않습니다.
참고: https://developer.mozilla.org/ko/docs/Web/HTML/Element/a#onclick_%EC%9D%B4%EB%B2%A4%ED%8A%B8
onClick={e => { | ||
e.preventDefault() | ||
}}> | ||
회사소개 ㅣ 채용안내 ㅣ 상품입점 및 제휴문의 ㅣ 고객센터 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
평행적인 요소는 모두 분리된 li 태그안에 담는 것이 좋습니다.
|
||
<div className="footer-sections"> | ||
<div className="footer-left"> | ||
<div className="footer-logo"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 태그는 필요없습니다.
어차피 img 하나만 감싸고 있으므로 스타일을 잘 적용하여 DOM depth를 줄이는 것이 좋습니다.
</ul> | ||
</div> | ||
|
||
<div className="footer-sections"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style 적용에 통일성이 부족합니다.
어떤 컴포넌트는 css, 어떤 컴포넌트는 css in js를 사용하는데, 한 프로젝트에서는 스타일을 전부 통일하는 것이 좋습니다.
navigate('/product/search') | ||
return | ||
} | ||
const header_LoginCheck = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수는 모두 camel case를 사용합니다.
컨벤션을 지키는 것도 중요합니다.
onChange={e => { | ||
if (item === 'price') { | ||
const { value } = e.target | ||
const onlyNumber = value.replace(/[^0-9]/g, '') | ||
setTest(onlyNumber) | ||
} else { | ||
setTest(e.target.value) | ||
} | ||
console.log(test) | ||
}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
함수는 위에서 선언해주세요.
가독성을 크게 해칩니다.
const [placeholderText, setPlaceholderText] = | ||
useState('추가할 은행을 선택해주세요.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
능동적으로 사용자의 선택에 의해 바뀌는 데이터가 아닌 경우 상수를 이용하는 것이 오타를 줄이고 재사용성을 높일 수 있습니다.
@@ -0,0 +1,446 @@ | |||
import React, { useEffect, useState, useRef } from 'react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일단 컴포넌트가 너무 거대합니다.
더 분할할수 있을지 고려해보고 작은 컴포넌트의 조합으로 만드는 작업이 필요해보입니다.
const [deliveryMessage, setDeliveryMessage] = useState('') | ||
const [postalCode, setPostalCode] = useState('') | ||
const [address, setAddress] = useState('') | ||
const [showPostcodeModal, setShowPostcodeModal] = useState(false) | ||
const [isLoggedIn, setIsLoggedIn] = useState(false) | ||
const [accountList, setAccountList] = useState([]) | ||
const [recipientName, setRecipientName] = useState('') | ||
const [phoneNumber, setPhoneNumber] = useState('') | ||
const [addressDetail, setAddressDetail] = useState('') | ||
const [isFormValid, setIsFormValid] = useState(false) | ||
const [selectAccount, setSelectAccount] = useState('') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isFormValid의 경우 다른 데이터에서 계산 가능한 값이고, 다른 state들도 하나의 객체로 관리할 수 있어 보입니다.
관리할 상태의 증가는 유지보수를 어렵게 하고 휴먼에러의 확률을 높입니다.
state는 최소한의 갯수를 유지하도록 고려해보세요.
const settings = { | ||
dots: true, | ||
infinite: true, | ||
speed: 1000, | ||
slidesToShow: 1, | ||
slidesToScroll: 1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 상수 데이터 역시 굳이 컴포넌트 안에서 선언할 필요가 없습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
총평
어려운 상황에서 적은 인원으로 프로젝트를 착실히 수행하시느라 수고하셨습니다.
아쉬운 점
배포된 프로덕트의 코드에 console.log가 너무 많이 남아있습니다.
코드 상에서는 console.log가 없도록 하는 것이 좋습니다.
또한, 어떤 컴포넌트에서는 css를 사용하고 어떤 컴포넌트에서는 css in js를 사용하는 것이 아쉽습니다.
프로젝트에서는 컨벤션을 맞추는 것이 아주 중요합니다.
state를 효율적으로 관리하지 못하는 부분이 조금 있습니다.
관리하는 state의 수를 최소화 하는 것이 유지보수 측면에서 유리합니다.
모두 고생하셨습니다.
패스트캠퍼스 FE 5기 10조 👆
프로젝트 소개 👀
기술 스택
Environment
Config
Development
Communication
팀원 소개
- 배너
- 카테고리
[카테고리 페이지]
[상세 페이지]
[결제 페이지]
[API정리]
[마이페이지]
- 계좌관리
[검색 결과 페이지]
<zustand 전역상태관리>
[회원가입 페이지]
[마이페이지]
- 주문 조회
- 회원정보 수정
[관리자 페이지]
- 유저 리스트
- 제품 등록
- 제품 관리
[장바구니 페이지]
<라우터 설정>
<GitHub 관리>
- 헤더
- 푸터
<자료조사>
페이지 기능
[메인페이지]
[로그인 페이지]
[회원가입 페이지]
[마이 페이지]
[카테고리 페이지]
[제품 상세 페이지]
[결제 페이지]
[마이 페이지]
<계좌관리>
<주문 조회>
<회원 정보 수정>
[관리자 페이지]
[장바구니 페이지]